Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch 6 #149

Closed
wants to merge 2 commits into from
Closed

Patch 6 #149

wants to merge 2 commits into from

Conversation

Katze2664
Copy link
Contributor

wrapper_args is a list of lists, not list of dicts

wrapper_args is a list of lists, not list of dicts
wrapper_args is a list of lists, not list of dicts
@AhmedMagdyHendawy
Copy link
Contributor

Thanks for pointing this out!

It should be a list of dict for a better argument passing. Now, it should work with dictionaries. Please let us know if this works better.

Cheers,
Ahmed

@Katze2664
Copy link
Contributor Author

Hi Ahmed,

Yep, that works.

It removes the ability to pass positional arguments, and provides 2 different ways to pass a dictionary of arguments (either in wrappers as a list of (Wrapper class, dictionary) tuples, or in wrappers_args as a list of dictionaries), but perhaps this redundancy is ok since it is probably better practice to pass keyword arguments rather than positional arguments.

Thanks!
Katze

@boris-il-forte
Copy link
Collaborator

I guess we can close this pull request, then, right?

@Katze2664 Katze2664 closed this Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants